-
Notifications
You must be signed in to change notification settings - Fork 75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Component signature #385
Component signature #385
Conversation
Adds backwards-compatible support for component `Signature`s per [Ember RFC #0748][rfc]. The public API of `Component` is now *more* permissive than it was previously, because it changes the type parameter to be an unconstrained generic `<S>` (for "signature") which can accept *either* the existing `Args` types *or* a new `Signature` which includes `Args` and adds `Blocks` and `Element`. [rfc]: emberjs/rfcs#748 The `Args` part of the new signature work exactly like the old args-only type did. The `Blocks` and `Element` parts of a signature are inert from the perspective of TypeScript users who are not yet using [Glint][glint], but Glint users will benefit directly once Glint releases an update which can requires a version of `@glimmer/component` incorporating this change. [glint]: https://github.com/typed-ember/glint Since this change is backwards compatible, we can deprecate the non-`Signature` form at a later time when we are ready for a 2.0 release. To validate these changes, with the relatively complicated type machinery they require under the hood, this also introduces the `expect-type` type testing library, rewrites the existing type tests using it, and introduces new type tests for all supported forms of the `Signature`.
Would love a review not only from core Glimmer folks but also from @dfreeman. |
interface FullLongSig { | ||
Args: { | ||
Named: Args; | ||
Positional: []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😢
LGTM |
This provides the expected resolution within a component, but does not introduce a breaking change by requiring all callers to pass a type parameter. Co-authored-by: James C. Davis <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great; thanks for getting it implemented @chriskrycho! Just a couple questions around type visibility that the answer to might be 'nope, this is exactly how we want it already'
Note for myself: looks like once we land this I'll need to back-port it to the v1.x branch so we can cut a v1.1 release. |
Co-authored-by: Dan Freeman <[email protected]>
Backport component signature (#385) to v1.x
Adds backwards-compatible support for component
Signature
s per Ember RFC #0748. The public API ofComponent
is now more permissive than it was previously, because it changes the type parameter to be an unconstrained generic<S>
(for "signature") which can accept either the existingArgs
types or a newSignature
which includesArgs
and addsBlocks
andElement
.The
Args
part of the new signature work exactly like the old args-only type did. TheBlocks
andElement
parts of a signature are inert from the perspective of TypeScript users who are not yet using Glint, but Glint users will benefit directly once Glint releases an update which can requires a version of@glimmer/component
incorporating this change.Since this change is backwards compatible, we can deprecate the non-
Signature
form at a later time when we are ready for a 2.0 release.To validate these changes, with the relatively complicated type machinery they require under the hood, this also introduces the
expect-type
type testing library, rewrites the existing type tests using it, and introduces new type tests for all supported forms of theSignature
.